-
-
Notifications
You must be signed in to change notification settings - Fork 258
Implement nullish coalescing operator in parser #761
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow amazing work @azz! I'm good with this if the name is good for everyone
For the tests, I would check https://github.com/babel/babylon/pull/742/files which is similar Could do chained so |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 fine with plugin name, and agreed on adding the tests @hzoo mentioned
cc @littledan |
Fine with merging now if we can get a PR for tests next, (logic shouldn't be any different) |
new PR #762, should probably be |
@@ -791,6 +791,7 @@ enum BinaryOperator { | |||
| "|" | "^" | "&" | "in" | |||
| "instanceof" | |||
| "|>" | |||
| "??" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a LogicalOperator
, not a BinaryOperator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the parser for the nullish coalescing operator,
a ?? b
, currently at stage 1.In the AST it isn an operator under
BinaryExpression
. Should it move toLogicalExpression
?Picked the plugin name:
"nullishCoalescingOperator"
. It's a bit of a mouthful, and may well change.Ref: babel/proposals#14
Proposal: https://github.com/gisenberg/proposal-null-coalescing
Proposal will likely be renamed from
nullary
tonullish
, see tc39/proposal-nullish-coalescing#3.I feel like I haven't added enough tests, but I'm not too sure of the edge cases I should test, let me know if you know of any!